New async rosapi params module implementation #1001
Conversation
rosapi/src/rosapi/params.py
Outdated
| from rosapi.proxy import get_nodes | ||
|
|
||
| from .async_helper import futures_wait_for |
There was a problem hiding this comment.
Can you use the fully qualified instead of relative path?
| client = _node.create_client( | ||
| SetParameters, | ||
| f"{node_name}/set_parameters", | ||
| callback_group=MutuallyExclusiveCallbackGroup(), |
There was a problem hiding this comment.
Did you also want this to be reentrant callback group?
There was a problem hiding this comment.
No need for service clients as they are created only for a single service call. In fact, if I understand it correctly, using the default callback group should be also ok here but, just in case I'm wrong, I create unique callback groups for each client.
| client = _node.create_client( | ||
| GetParameters, | ||
| f"{node_name}/get_parameters", | ||
| callback_group=MutuallyExclusiveCallbackGroup(), |
| @@ -1,4 +1,5 @@ | |||
| string name | |||
| string default_value | |||
There was a problem hiding this comment.
The PR says that the extra response fields are non-intrusive as you can just ignore them, but removing this default value probably will affect a few users.
I think it makes sense to remove it since you're just getting a parameter, and can handle falling back to a default on the client side, but just pointing out this is another breakage.
There was a problem hiding this comment.
AFAIK roslibjs does not use this feature but you're right that it still may affect some users, so I restored this field.
Public API Changes
The
set_param,get_param, anddelete_paramservice types now include:These changes do not break the compatibility with the current version of
roslibjsclient, as it will ignore these new fields. Additionally, to make it backward compatible, ifget_paramfails, the value field in the response will still be set to an empty JSON string.Description
Changed the implementation of rosapi params module to take advantage of async/await feature of rclpy executor instead of creating a separate node.
Aside from the aforementioned API change, there are few implementation changes:
get_param_namescallslist_parametersservice for each node in parallel, not sequentially. This should make it much more robust, especially if the node graph is quite big.ReentrantCallbackGroupwhich means multiple service callbacks can be processed asynchronously.ros2paramcli package are no longer used as they are not awaitable.